Skip to content

treewide: use SEC_TAG_TLS_INVALID for unset security tag #24020

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 20, 2025
Merged

treewide: use SEC_TAG_TLS_INVALID for unset security tag #24020

merged 1 commit into from
Aug 20, 2025

Conversation

maxd-nordic
Copy link
Contributor

@maxd-nordic maxd-nordic commented Aug 15, 2025 •
edited
Loading

This is another attempt of #23571 to fix handling sec_tags that have not been set.

The fact that the modem uses uint32_t for sec_tags, while Zephyr uses int, is a bit unfortunate.
Often, -1 or 0 are used to indicate an invalid sec_tag, and it is checked whether a sec_tag is positive.

However, there are some "debug" sec_tags for the modem, that register as negative values, while being valid. To avoid confusion, use a proper placeholder for an invalid sec_tag.

@maxd-nordic maxd-nordic requested review from a team as code owners August 15, 2025 13:51
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Aug 15, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Aug 15, 2025 •
edited
Loading

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 5

Inputs:

Sources:

sdk-nrf: PR head: ab76b37bfd0f13f97b65bf4e231070cc2e8ab260

more details

sdk-nrf:

PR head: ab76b37bfd0f13f97b65bf4e231070cc2e8ab260
merge base: 734f2ae80e90c956b030785e3c6242f5e8ef0b46
target head (main): 734f2ae80e90c956b030785e3c6242f5e8ef0b46
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (19) 
applications
│  ├── serial_lte_modem
│  │  ├── src
│  │  │  ├── ftp_c
│  │  │  │  │ slm_at_ftp.c
│  │  │  ├── http_c
│  │  │  │  │ slm_at_httpc.c
│  │  │  ├── mqtt_c
│  │  │  │  │ slm_at_mqtt.c
│  │  │  ├── nativetls
│  │  │  │  │ slm_native_tls.c
│  │  │  ├── slm_at_fota.c
│  │  │  ├── slm_at_socket.c
│  │  │  ├── slm_at_tcp_proxy.c
│  │  │  ├── slm_at_udp_proxy.c
│  │  │  │ slm_defines.h
doc
│  ├── nrf
│  │  ├── releases_and_maturity
│  │  │  ├── releases
│  │  │  │  │ release-notes-changelog.rst
samples
│  ├── cellular
│  │  ├── http_update
│  │  │  ├── application_update
│  │  │  │  ├── src
│  │  │  │  │  │ main.c
│  │  │  ├── modem_delta_update
│  │  │  │  ├── src
│  │  │  │  │  │ main.c
│  │  │  ├── modem_full_update
│  │  │  │  ├── src
│  │  │  │  │  │ main.c
│  │  ├── modem_shell
│  │  │  ├── src
│  │  │  │  ├── rest
│  │  │  │  │  │ rest_shell.c
│  │  │  │  ├── sock
│  │  │  │  │  │ sock_shell.c
subsys
│  ├── net
│  │  ├── lib
│  │  │  ├── aws_fota
│  │  │  │  ├── src
│  │  │  │  │  │ aws_fota.c
│  │  │  ├── fota_download
│  │  │  │  ├── src
│  │  │  │  │  ├── fota_download.c
│  │  │  │  │  ├── util
│  │  │  │  │  │  │ fota_download_util.c
│  │  │  ├── ftp_client
│  │  │  │  ├── src
│  │  │  │  │  │ ftp_client.c

Outputs:

Toolchain

Version: c5be9c56c7
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:c5be9c56c7_bba2ea5f2e

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 365
  • ✅ Integration tests
    • ✅ test-fw-nrfconnect-nrf-iot_cloud
    • ✅ test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • ✅ test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • ✅ test-fw-nrfconnect-nrf-iot_samples
    • ✅ test-fw-nrfconnect-nrf_lrcs_mosh
    • ✅ test-sdk-dfu
    • ⚠️ test-fw-nrfconnect-nrf_lrcs_mosh
Disabled integration tests
    • test-fw-nrfconnect-nrf_lrcs_positioning
    • desktop52_verification
    • doc-internal
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-proprietary_esb
    • test-fw-nrfconnect-ps-main
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-low-level
    • test-sdk-audio
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 15, 2025 •
edited
Loading

You can find the documentation preview for this PR here.

Preview links for modified nRF Connect SDK documents:

https://ncsdoc.z6.web.core.windows.net/PR-24020/nrf/releases_and_maturity/releases/release-notes-changelog.html

@SeppoTakalo
Copy link
Contributor

Looks like most modules should now include zephyr/net/tls_credentials.h to build correctly.

This is where the SEC_TAG_TLS_INVALID is defined.

@maxd-nordic maxd-nordic force-pushed the invalid-sec-tag-2 branch 2 times, most recently from c05a3f0 to a63b663 Compare August 18, 2025 15:31
@maxd-nordic maxd-nordic requested a review from a team as a code owner August 18, 2025 15:31
@github-actions github-actions bot added doc-required PR must not be merged without tech writer approval. and removed changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Aug 18, 2025
peknis
peknis requested changes Aug 19, 2025
View reviewed changes
@maxd-nordic
treewide: use SEC_TAG_TLS_INVALID for unset security tag
ab76b37 
This is another attempt of
nrfconnect#23571 to fix handling
sec_tags that have not been set.

The fact that the modem uses uint32_t for sec_tags,
while Zephyr uses int, is a bit unfortunate.
Often, -1 or 0 are used to indicate an invalid sec_tag,
and it is checked whether a sec_tag is positive.

However, there are some "debug" sec_tags for the modem,
that register as negative values, while being valid.
To avoid confusion, use a proper placeholder for an invalid sec_tag.

Signed-off-by: Maximilian Deubel <[email protected]>
@maxd-nordic maxd-nordic requested a review from peknis August 19, 2025 07:06
@nordicjm nordicjm merged commit 2efd6d7 into nrfconnect:main Aug 20, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tokangas tokangas tokangas approved these changes

@SeppoTakalo SeppoTakalo SeppoTakalo approved these changes

@jtguggedal jtguggedal jtguggedal approved these changes

@rlubos rlubos rlubos approved these changes

@trantanen trantanen trantanen approved these changes

@peknis peknis peknis approved these changes

@eivindj-nordic eivindj-nordic eivindj-nordic approved these changes

@divipillai divipillai divipillai approved these changes

Assignees
No one assigned
Labels
doc-required PR must not be merged without tech writer approval.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.